[CORE-8485] Reset translation state on snapshot #24522
Conversation
Signed-off-by: Michał Maślanka <michal@redpanda.com>
Signed-off-by: Michał Maślanka <michal@redpanda.com>
When an stm receives Raft snapshot it indicates the whole in memory state of that state machine should be replaced with the state from the snapshot. The datalake translation state machine was incorrectly handling raft snapshot which lead to its state being out of date after the snapshot is applied. Raft snapshot for translation_stm is empty so the correct action is to reset the state machine state and wait for the update to be applied. Fixes: CORE-8485 Signed-off-by: Michał Maślanka <michal@redpanda.com>
| // state machine will not hold any obsolete state that should be overriden | ||
| // with the snapshot. | ||
| vlog(_log.debug, "Applying raft snapshot, resetting state"); | ||
| _highest_translated_offset = kafka::offset{}; |
There was a problem hiding this comment.
A bit unclear to me why is it okay to throw out the offset like this - as the raft snapshot is empty, STMs on different replicas will necessarily get out of sync. Is it because, if the translation is to be continued from some later point, we hope to get another update in the log?
There was a problem hiding this comment.
another update is one thing, the other one is the reconciliation with the datalake coordinator which happens before every translation. The empty snapshot indicates the snapshot is not required by this STM, hence resetting the state here is the only viable option we have
There was a problem hiding this comment.
The empty snapshot indicates the snapshot is not required by this STM, hence resetting the state here is the only viable option we have
Yes, but why is it okay to have an empty snapshot?
There was a problem hiding this comment.
I was wondering about that, and given that we always commit to the coordinator i think it is safe. Am I right @bharathv ?
There was a problem hiding this comment.
I think its ok to reset. Currently this offset is only used to enforce max_collectible_offset on the replica. It is ok to reset because lowering the max collectible offset only delays compaction and has no correctness implications until it catches up again. As Michal said the leader reconciles with the coordinator every time to get the offset_to_translate_from.
@WillemKauf is planning to get rid of translation in this path for read replicas, so we could probably just store the log offset and avoid this kafka offset altogether. This kafka offset was added as an optimization so the coordinator can avoid reconciliation in every round of translation but since then that optimization has been removed to simplify the code, or we could just store a pair of <kafka_offset, log_offset> since it is already in serde and implement the optimization later.
There was a problem hiding this comment.
Currently this offset is only used to enforce max_collectible_offset on the replica
is there a reason to be concerned that the scope could increase, this assumption no longer holds, and now there is a problem?
There was a problem hiding this comment.
hmm can't think of a scope increase for the offset in the near future. The only reason it exists is to enforce max_collectible_offset. Also as noted, the plan is to get rid of offset translation in this path altogether and make this translation state self contained, and that automatically fixes this problem too.
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59592#0193b5f5-a10e-4615-a139-eb2123b0e78e |
|
/backport v24.3.x |
|
Failed to create a backport PR to v24.3.x branch. I tried: |
mmaslankaprv
deleted the
CORE-8485-reset-translation-state-on-snapshot
branch
When an stm receives Raft snapshot it indicates the whole in memory
state of that state machine should be replaced with the state from the
snapshot. The datalake translation state machine was incorrectly
handling raft snapshot which lead to its state being out of date after
the snapshot is applied. Raft snapshot for
translation_stmis empty sothe correct action is to reset the state machine state and wait for the
update to be applied.
Fixes: CORE-8485
Backports Required
Release Notes